fix: allow screenshot capture when filename is an empty string #37678
Conversation
Adds test case to verify that browser_take_screenshot handles empty filename argument by generating a default timestamped filename. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Adds z.preprocess to convert empty string to null in filename validation, allowing empty filename to fall back to default timestamp-based naming. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
yury-s
left a comment
yury-s
left a comment
There was a problem hiding this comment.
When is it called with an empty string instead of omitting the parameter?
| const screenshotSchema = z.object({ | ||
| type: z.enum(['png', 'jpeg']).default('png').describe('Image format for the screenshot. Default is png.'), | ||
| filename: z.string().optional().describe('File name to save the screenshot to. Defaults to `page-{timestamp}.{png|jpeg}` if not specified.'), | ||
| filename: z.preprocess(v => { |
There was a problem hiding this comment.
just change ?? to || at line 54:
const fileName = await tab.context.outputFile(params.filename || dateAsFileName(fileType), { origin: 'llm', reason: 'Saving screenshot' }); 
|
@yury-s Thanks for reviewing! I'm using this MCP server with the GPT-5 model via Mastra. When the filename is omitted in the prompt, the model sometimes calls the function with filename: "", which appears to be a model hallucination. While switching models is a possible workaround, I suggest that this edge case be handled within the library for improved robustness. |
|
Hi @yury-s I'm just gently bumping this PR. No rush, but I would appreciate it if you could take a look at the latest changes when you get a chance. Thanks for your time! |
|
Please start with filing an issue, it is unclear why LLM is passing empty string as a filename, we might want to push back there. |
|
@pavelfeldman Thanks for the heads-up. I’ve opened an issue to track this: microsoft/playwright-mcp#1138 |
Test results for "MCP"2570 passed, 108 skipped Merge workflow run. |
3 participants
Problem
file path for is outsidereturning an error response instead of an image.Root Cause
Fix
page-YYYY-MM-DDTHH-mm-ss-SSSZ.png) in the output directory.Behavior Changes